Skip to content

fix(storage): fix system test and change scope for iam access token#47

Merged
crwilcox merged 6 commits intogoogleapis:masterfrom
HemangChothani:storage_fix_system_test
Feb 12, 2020
Merged

fix(storage): fix system test and change scope for iam access token#47
crwilcox merged 6 commits intogoogleapis:masterfrom
HemangChothani:storage_fix_system_test

Conversation

@HemangChothani
Copy link
Copy Markdown
Contributor

Fixes #46

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 6, 2020
@HemangChothani
Copy link
Copy Markdown
Contributor Author

HemangChothani commented Feb 6, 2020

System test throws 'Identity and Access Management (IAM) API has not been used in project before or it is disabled. ' error, so need (IAM) permission for this project.

@HemangChothani HemangChothani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
tests/system.py Outdated
service_account_email = Config.CLIENT._credentials.service_account_email
name = client.service_account_path("-", service_account_email)
scope = ["https://www.googleapis.com/auth/devstorage.read_write"]
scope = ["https://www.googleapis.com/auth/cloud-platform"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a lot of access. @jkwlui @frankyn is this scope needed for this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(we don't have to block merging on this as it is a test, but if this is needed it seems like a large scope for narrow use?)

Copy link
Copy Markdown
Contributor

@tseaver tseaver Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I can confirm that both tests fail on master with a 403 without this patch.

Update: it fails even with the cloud-platform scope for me on master.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you narrow the scope down to: https://www.googleapis.com/auth/iam

Documented at the bottom of the following document: https://cloud.google.com/iam/docs/reference/credentials/rest/v1/projects.serviceAccounts/signBlob

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, you can do join both:

scope = ['https://www.googleapis.com/auth/devstorage.read_write', 'https://www.googleapis.com/auth/iam']

@HemangChothani HemangChothani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@crwilcox crwilcox merged commit bc5375f into googleapis:master Feb 12, 2020
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…oogleapis#47)

* fix(storage): change scope for iam access token

* fix: narrow scope

* fix: trailing commas

* chore: blacken

Co-authored-by: Christopher Wilcox <crwilcox@google.com>
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…oogleapis#47)

* fix(storage): change scope for iam access token

* fix: narrow scope

* fix: trailing commas

* chore: blacken

Co-authored-by: Christopher Wilcox <crwilcox@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storage: access token field is unused in system test 'test_create_signed_read_url_*_w_access_token'

6 participants